feat(sandbox): use policy-first OCI image identity - #2509
feat(sandbox): use policy-first OCI image identity#2509matthewgrossman wants to merge 12 commits into
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-2509.docs.buildwithfern.com/openshell |
E2E Test AttestationLocal E2E tests passed. CI does not currently run E2E tests, so this comment serves as the verification record.
Test SummaryTests Executed
Podman E2E was not run because Podman is not installed on this host. Podman unit and driver suites passed under |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331, and the author has maintainer authority.
Head SHA: 3bf0b9f6f5456092938e711dc35a896d937110cd
Review findings:
- Blocking sandbox identity regression found inline.
- Numeric OCI identity edge case found inline.
- Fern docs were partially updated, but public tutorials/examples still describe
process.run_as_user: sandbox/run_as_group: sandboxas preserving default process settings indocs/get-started/tutorials/first-network-policy.mdx,docs/get-started/tutorials/github-sandbox.mdx, andexamples/sandbox-policy-quickstart/*. With omission now carrying driver fallback semantics, update those snippets to omitprocessor clearly scope explicitsandboxto the base image.
Docs: Updates are present, but the tutorial/example gap above remains blocking for this direct user-facing policy behavior change.
Next state: gator:in-review
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Updated E2E Test AttestationLocal E2E tests passed after addressing review feedback.
Podman E2E remains unavailable because Podman is not installed on this host. Podman unit and driver coverage passed under |
|
Addressed the remaining documentation finding in |
BlockedHead SHA: Gator is blocked because PR #2509 is still marked as a draft. I saw @matthewgrossman's update that Next action: @matthewgrossman should mark the PR ready for review when it is ready for gator to re-check the latest head and continue CI/test monitoring. |
|
gator I summon you! |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: f163130351d232d7c47417c76317c262c6473409
Thanks @matthewgrossman, I checked your f1631303 update against the prior gator findings. The explicit policy-field and numeric UID:GID issues appear addressed, and the PR is no longer draft.
Review findings:
- Warning found inline: OCI name validation and the later privilege-drop lookup can still diverge.
- Docs note found inline: one quickstart comment and matching tutorial prose still mention preserving process settings after the examples removed
process:.
Docs: Fern docs were updated, but the stale wording above should be corrected with the review feedback.
Next state: gator:in-review
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: a229ecba987c3dc1700979391495d7d8f5b5e7cf
Thanks @matthewgrossman, I re-checked the latest head after your update and the prior gator findings.
Review findings:
- No blocking findings remain. The independent code review found no actionable issues on the current head.
Docs: Fern docs and quickstart examples were updated for the direct policy/runtime UX change; no navigation update appears needed.
E2E: test:e2e is required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.
Next state: gator:watch-pipeline
|
Label |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 41502ad0a42b237ca5512c9f9afd47b4206b8280
Thanks @matthewgrossman, I re-checked the latest head after the Kubernetes/OpenShift identity protection update. The prior runtime-boundary warning appears addressed, and the current-head independent review found one remaining example correctness warning inline.
Review findings:
- Warning found inline: the BYOC example now declares
USER app, but/sandboxremains root-owned.
Docs: Fern docs and related agent skills were updated for the direct policy/runtime UX change; no navigation update appears needed.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.
Next state: gator:in-review
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 5ca344e6ecc9e233933075e3fb2707e0083571b4
Thanks @matthewgrossman, I re-checked the latest head after the workspace-root refactor. The prior workspace hard-link concern appears addressed by owning only the workspace root, but the current-head independent review found two remaining privilege-drop issues inline.
Review findings:
- Critical found inline: supplementary groups can survive the UID/GID drop for explicit numeric or partial OCI identity paths.
- Critical found inline: no-driver identity resolution can leave a group-only policy running as UID 0.
Docs: Fern docs and related agent skills were updated for the direct policy/runtime UX change; no navigation update appears needed.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.
Next state: gator:in-review
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 7efabcab54fa3d233a3ce2e7a4a38a9f7ec583ec
Thanks @matthewgrossman, I re-checked the latest head after the partial identity-drop hardening. The prior supplementary-group and no-driver UID 0 findings appear addressed, but the current-head independent review found remaining E2E and example-policy issues.
Review findings:
- Warnings found inline: Podman-selected E2E tests still use the CLI
--from <Dockerfile>path, which builds through Docker defaults instead of the Podman harness socket. - Warning: several shipped example policies still explicitly set
run_as_user: sandbox/run_as_group: sandbox(examples/agent-driven-policy-management/policy.template.yaml,examples/local-inference/sandbox-policy.yaml,examples/multi-agent-notepad/policy.template.yaml,examples/governance-interceptor/policy.yaml, andexamples/governance-interceptor/smoke.sh). That disables the new OCI fallback and will fail for custom images without asandboxaccount unless those examples intentionally require the legacy identity.
Docs: Fern docs and the related policy/CLI/debug skills were updated for the main behavior change, but the example-policy drift above remains.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.
Next state: gator:in-review
|
Addressed the latest Gator findings in c60fae3:
Validation: e2e-podman and e2e-docker feature builds pass; focused Docker custom_image and driver_config_volume E2E pass (5/5); mise run pre-commit and mise run test pass. |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: c60fae323b5d45339f5e05f37fdfacf3e1c8159e
Thanks @matthewgrossman, I re-checked the latest head after your c60fae32 update. The prior Podman E2E selection and generic example-policy findings appear addressed, but the current-head independent review found remaining runtime-validation and docs warnings inline.
Review findings:
- Warnings found inline: the explicit/default process identity validation path now accepts arbitrary nonzero numeric UID/GID values instead of preserving the prior sandbox ID range for non-OCI inputs.
- Warning found inline: the policy schema docs now understate explicit identity validation by saying only
root/0are rejected.
Docs: Fern docs and related agent skills were updated for the main policy-first OCI behavior, but the explicit-policy validation wording still needs the inline correction.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending for this head.
Next state: gator:in-review
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 30cd1eca9cb0d7d1f96671990fab6e1aa9ba36f4
Thanks @matthewgrossman, I re-checked the latest head after your 30cd1eca update. The prior explicit/default UID/GID validation and policy-schema documentation findings appear addressed, but the current-head independent review found two remaining documentation precision warnings inline and one test coverage warning.
Review findings:
- Warnings found inline: two docs pages still say explicit process identity only rejects
root/0, which understates the restored sandbox-range constraint. - Warning: Podman OCI identity has unit coverage, but no end-to-end coverage exercises a real Podman inspect/create/immutable-image launch path. A Podman E2E or driver integration test would reduce risk around API schema and image-ID behavior.
Docs: Fern docs were updated for the main policy-first OCI behavior, but the two inline docs corrections remain.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending for this head.
Next state: gator:in-review
|
Addressed the remaining Gator findings in
Local validation: |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 23c198b695b33ba1c531325573523ecb48c353d5
Thanks @matthewgrossman, I re-checked the latest head after your 23c198b6 update. The prior documentation precision and Podman E2E coverage findings appear addressed, and the current-head independent review found no actionable issues.
Review findings:
- No blocking findings remain. The independent code review found no actionable issues on the current head.
Docs: Fern docs cover the direct policy/runtime UX change, including policy schema, compute drivers, policies, and security guidance; no navigation update appears needed.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and Helm Lint are green; required E2E is still pending.
Next state: gator:watch-pipeline
|
Follow-up |
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 6d5344159869569068663a36208624ddc2ae0a4f
Thanks @matthewgrossman, I re-checked the latest head after your 6d534415 Podman E2E fix. The prior Podman coverage concern appears addressed: the new test builds through the selected Podman engine, verifies OCI Config.User, supplies a policy that intentionally omits process, and checks the launched sandbox container image ID.
Review findings:
- No blocking findings remain. The independent code review found no actionable issues on the current head.
Docs: Fern docs and related examples cover the direct policy/runtime UX change; no navigation update appears needed.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending for this head.
Next state: gator:watch-pipeline
Closes #2331 Preserve per-field policy omission, derive Docker and Podman fallbacks from the inspected immutable image, and resolve the final numeric identity before starting agent children. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Keep explicit policy values and OCI-declared names intact, defer passwd lookup until a primary GID is required, and refresh stale policy examples. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
6d53441 to
254364a
Compare
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 254364a2fab50b83c2ceb847b8f4b183bfbc4801
Thanks @matthewgrossman, I re-checked the latest head after the Podman E2E follow-up and compared it with the prior gator review ledger. The previously resolved findings remain addressed.
Review findings:
- No blocking findings remain. The independent code review found no actionable issues on the current head.
Docs: Fern docs, examples, and related agent skills cover the direct policy/runtime UX change; no navigation update appears needed.
E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending; Helm Lint and GPU E2E are green.
Next state: gator:watch-pipeline
Summary
Removes Docker and Podman's dependency on a baked-in
sandboxaccount.Explicit policy identity fields win independently; omitted fields fall back to
the final image's OCI
Config.User, which is validated inside the image beforeany agent child starts.
This PR is intentionally limited to process identity. It does not create,
chown, or otherwise prepare
/sandboxfor OCI-derived identities. OCIWorkingDirand dynamic workspace-root support remain in follow-up issue#2526.
Related Issue
Closes #2331
Changes
run_as_user/run_as_groupomission for Docker andPodman while retaining legacy
sandboxdefaults for Kubernetes, OpenShift,VM, and unknown/remote drivers.
identity metadata, and create the container from the inspected immutable
image ID.
policy values and OCI-declared name or numeric components.
passwd/group access for accountless numeric
UID:GID.existing privilege-drop path.
Kubernetes/OpenShift retain inherited platform groups.
launching the inspected immutable image.
readiness, and gateway protocols unchanged.
the narrowed scope.
Scope change
An earlier revision attempted to create and change ownership of
/sandboxforOCI-derived identities. That behavior and its recursive/nested-mount tests were
removed: it expanded an identity PR into a partial workspace implementation.
The complete workspace solution will use OCI
WorkingDirand is tracked in#2526.
Deviations from Plan
rewritten numerically. This preserves the image's declared account semantics
while still rejecting unknown, ambiguous, malformed, and root identities.
/sandboxpreparation was removed after review and deferred tothe coherent workspace-root follow-up.
Testing
mise run pre-commitpassesmise run testpassesTests added or updated:
immutable-image pinning and protected metadata; explicit and OCI-declared
name preservation; lazy passwd lookup; named, numeric, mixed, missing,
malformed, ambiguous, and root OCI identity handling; direct/SSH identity
material containment; Kubernetes/OpenShift compatibility.
of
mise run test.e2e/rust/tests/custom_image.rscovers a named OCI user/group and apasswd-less numeric
UID:GID; both focused Docker-backed tests pass locally.Podman unit and driver suites pass. Podman E2E was not run because Podman is not
installed on this host.
Checklist
Documentation updated:
architecture/compute-runtimes.md